Skip to content

Allow configuring the script filename casing rule - #78119

Merged
akien-mga merged 1 commit into
godotengine:masterfrom
RedMser:script-filename-casing
Mar 5, 2024
Merged

Allow configuring the script filename casing rule#78119
akien-mga merged 1 commit into
godotengine:masterfrom
RedMser:script-filename-casing

Conversation

@RedMser

@RedMser RedMser commented Jun 11, 2023 •
edited by raulsntos
Loading

Copy link
Copy Markdown
Contributor

Supersedes #41991
Closes godotengine/godot-proposals#1211
Bugsquad edit: Closes godotengine/godot-proposals#5956

  • Add a project setting editor/naming/script_name_casing which works similar to the scene equivalent.
    • Defaults to "Auto", which detects the casing based on the preference of the currently selected language (C# for example prefers PascalCase whereas GDScript prefers snake_case).
  • Also simplify the code a bit. It seemed to do path reformatting logic in both config and _language_changed. And the logic was also overly complex, with the amount of helper methods available to manipulate strings as file paths.
  • Finally, add kebab-case to editor/naming/scene_name_casing for consistency, as it has also been added for script name.

TODO

  • Add kebab casing, since some languages like JavaScript and TypeScript would need this.
  • Test the logic with multiple languages in the dropdown (I'd be thankful if someone who's running Linux could try the Mono build).
  • Find out what get_recognized_extensions is and how it differs from get_extension. One is for the resource loader/saver, the other is for the script language.
  • ScriptLanguageExtension changes cause an error that I can not figure out. Needed to bind the enum, fixed.

@RedMser
RedMser requested review from a team as code owners June 11, 2023 16:30
@YuriSizov YuriSizov added this to the 4.x milestone Jun 12, 2023
@RedMser RedMser mentioned this pull request Jul 10, 2023
Closed
@nyxkn nyxkn mentioned this pull request Jul 11, 2023
Closed
@raulsntos raulsntos mentioned this pull request Sep 19, 2023
Closed
@KoBeWi

KoBeWi commented Sep 30, 2023

Copy link
Copy Markdown
Member

There is a minor issue where adding script to Node2D with pascal case will name the script Node2d. Not sure how to fix it best tbh. Probably it's not important, because scripts are supposed to have different name.

KoBeWi
KoBeWi reviewed Sep 30, 2023
View reviewed changes
Comment thread core/object/script_language.h Outdated
KoBeWi
KoBeWi reviewed Sep 30, 2023
View reviewed changes
Comment thread editor/script_create_dialog.cpp Outdated
KoBeWi
KoBeWi reviewed Sep 30, 2023
View reviewed changes
Comment thread core/object/script_language_extension.h Outdated
@fire

fire commented Oct 2, 2023

Copy link
Copy Markdown
Member

Do you think this can be in for Godot Engine 4.2?

@KoBeWi

KoBeWi commented Oct 2, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

We are in feature freeze, so probably not.
But the change seems rather safe.

@RedMser
RedMser force-pushed the script-filename-casing branch 2 times, most recently from 21d079e to 882eb30 Compare February 8, 2024 14:21
@RedMser
RedMser requested a review from KoBeWi February 8, 2024 14:56
@fire

fire commented Feb 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Do you think this can be in for Godot Engine 4.3?

Edited: There are some github actions problems preventing merge.

@RedMser

RedMser commented Feb 12, 2024

Copy link
Copy Markdown
Contributor Author

There are some github actions problems preventing merge.

It does not seem related to my changes, so I assume it's a problem with the tests.
I don't see an option to retry the pipeline, so I will do another rebase to latest master.

@RedMser
RedMser force-pushed the script-filename-casing branch from 882eb30 to 31ceac0 Compare February 12, 2024 17:43
@raulsntos

raulsntos commented Feb 23, 2024

Copy link
Copy Markdown
Member

Tried the Mono build on Linux and found these issues:

  • Since the logic is no longer re-executed when changing the language, and GDScript is the default language, the first time a user creates a C# script they have to manually fix the script path.
    image
  • When creating a new script from the attach button, converting node names like PascalCaseNode, snake_case_node, and kebab-case-node all works when the selected language is GDScript (it converts them all to snake_case). But when the selected language is C#, it fails to convert kebab-case to PascalCase (the others were converted successfully to PascalCase).
    image

@RedMser
RedMser force-pushed the script-filename-casing branch from 31ceac0 to 3cf51c6 Compare February 28, 2024 21:08
@RedMser

RedMser commented Feb 28, 2024

Copy link
Copy Markdown
Contributor Author

@raulsntos Thanks for testing! I believe I've fixed both issues, as well as cleaning up the code further (it had a confusingly named variable "current_language" which contained outdated values after my changes).

@fire
fire requested a review from a team February 29, 2024 01:48
raulsntos
raulsntos approved these changes Feb 29, 2024
View reviewed changes

@raulsntos raulsntos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected now, thank you!

AThousandShips
AThousandShips approved these changes Feb 29, 2024
View reviewed changes
Comment thread core/object/script_language.h Outdated
Comment thread editor/editor_node.h Outdated
Comment thread core/object/script_language.h Outdated
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Feb 29, 2024
@RedMser @akien-mga
Allow configuring the script filename casing rule
2bd714e 
Defaults to "Auto", which detects the casing based on the
preference of the currently selected language (C# for example
prefers PascalCase whereas GDScript prefers snake_case).
@akien-mga
akien-mga force-pushed the script-filename-casing branch from 3cf51c6 to 2bd714e Compare March 5, 2024 08:43
@akien-mga
akien-mga merged commit 897e2d9 into godotengine:master Mar 5, 2024
@akien-mga

akien-mga commented Mar 5, 2024

Copy link
Copy Markdown
Member

Thanks!

@akien-mga akien-mga mentioned this pull request Mar 5, 2024
Closed
@KoBeWi KoBeWi mentioned this pull request Apr 27, 2024
Closed
@tetrapod00 tetrapod00 mentioned this pull request Sep 12, 2024
Open
@tetrapod00 tetrapod00 mentioned this pull request Nov 9, 2024
Merged
@priahoud priahoud mentioned this pull request Nov 23, 2024
Closed
BendyLand pushed a commit to BendyLand/voltaire that referenced this pull request Aug 2, 2026
@akien-mga
Merge pull request godotengine#78119 from RedMser/script-filename-casing
4ce4b0a 
Allow configuring the script filename casing rule
BendyLand pushed a commit to BendyLand/voltaire that referenced this pull request Aug 2, 2026
@akien-mga
Merge pull request godotengine#78119 from RedMser/script-filename-casing
f5ed9d7 
Allow configuring the script filename casing rule
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@raulsntos raulsntos raulsntos approved these changes
@AThousandShips AThousandShips AThousandShips approved these changes
@KoBeWi KoBeWi Awaiting requested review from KoBeWi

Assignees

No one assigned

Labels

enhancement topic:editor usability

Projects

None yet

Milestone

4.3

7 participants

@RedMser @KoBeWi @fire @raulsntos @akien-mga @AThousandShips @YuriSizov